Code Review

Thanks for this — the SDK is well-structured overall: clean error hierarchy, good use of httpx, sensible config, and solid integration tests. That said, there are a few issues that need addressing before merge, including one security concern and a bug.

BUG-1 — NameError in loader.py if MpakClient() raises

File: packages/sdk-python/src/mpak/loader.py

client is assigned inside the try block. If MpakClient() itself raises, client is never bound. The except Exception handler then calls sys.exit(1), which raises SystemExit, triggering the finally block — which calls client.close() and throws NameError on the unbound name.

try:
    client = MpakClient()   # if THIS raises...
    ...
except Exception as e:
    sys.exit(1)
finally:
    client.close()          # NameError — client is unbound

Fix: Initialize client = None before the try and guard the close with if client: client.close().

SECURITY-1 — Zip slip vulnerability in client.py:load_bundle_from_url

File: packages/sdk-python/src/mpak/client.py

zipfile.ZipFile.extractall() does not sanitize entry paths. A zip containing entries like ../../etc/cron.d/evil will write files outside of dest_path. SHA256 verification confirms the zip hasn't been tampered with in transit, but does not protect against a malicious zip served from a compromised registry or CDN with a valid pre-computed hash.

with zipfile.ZipFile(bundle_path, "r") as zf:
    zf.extractall(dest_path)   # no path sanitization

Fix: Validate each entry path before extracting:

resolved_dest = dest_path.resolve()
for member in zf.namelist():
    if not (resolved_dest / member).resolve().is_relative_to(resolved_dest):
        raise ValueError(f"Zip slip attempt detected: {member}")
zf.extractall(dest_path)

DEAD-CODE-1 — Unreachable 404 branch in client.py:get_bundle_download

File: packages/sdk-python/src/mpak/client.py

The explicit if response.status_code == 404 check fires before response.raise_for_status(), so a 404 raises MpakNotFoundError and exits the try block immediately. The except httpx.HTTPStatusError handler's own 404 branch is therefore unreachable dead code.

if response.status_code == 404:
    raise MpakNotFoundError(...)   # caught here, exits try
response.raise_for_status()
...
except httpx.HTTPStatusError as e:
    if e.response.status_code == 404:   # never reached
        raise MpakNotFoundError(...) from e

One of the two should be removed.

DESIGN-1 — Bundle download bypasses the configured client

File: packages/sdk-python/src/mpak/client.py

Registry API calls use self._client (configured with User-Agent, Accept, follow_redirects). The actual bundle download creates a raw ad-hoc stream, silently losing those headers:

with httpx.stream("GET", url, follow_redirects=True, timeout=self.config.timeout) as response:

Fix: Use self._client.stream(...) for consistency.

DESIGN-2 — Version "0.1.0" hardcoded in two places independently

Files: src/mpak/__init__.py, src/mpak/types.py

__version__ in __init__.py and the user_agent default in MpakClientConfig both hardcode "0.1.0" independently of pyproject.toml. When the version is bumped, the user agent will silently report the wrong version.

Fix: Derive it at runtime: importlib.metadata.version("mpak").

Minor issues

• verify_integrity_bytes in _integrity.py — defined and exported but never called anywhere. Dead code or planned for future use?
• mock_registry_url fixture in tests/conftest.py — defined but never used in any test file.
• PRODUCT.md monorepo layout — shows sdk/ (old name) and python-sdk/ (wrong name). Should be sdk-typescript/ and sdk-python/ to match actual directories.
• pytest-asyncio in dev deps — listed as a dependency and asyncio_mode = "auto" is configured, but no async tests exist.
• generate-types.py string replacement — content.replace(" EmailStr,\n", "") hardcodes a 4-space indent assumption; will silently fail if the code generator changes its output format.
• generate-types.py double fetch — the OpenAPI spec is fetched once via urllib.request.urlopen (to print byte count) and then again by datamodel-codegen --url. The first fetch is redundant.

Re-review

All findings addressed except one minor item. Solid work — the fixes are correct and well-tested.

Status

MINOR-5 — Brittle EmailStr replacement in generate-types.py (still open)

The hardcoded 4-space indent assumption in the post-processing step is still there:

content = content.replace("    EmailStr,\n", "").replace("EmailStr", "str")

This will silently fail to remove the EmailStr import if datamodel-codegen ever changes its output formatting. Low risk since the generated file is checked in and would be caught on visual inspection, but worth a regex or AST-based replacement at some point.